Introduce Record class.
#72
Conversation
Just add directory to source set instead of replacing the whole set.
Still exposes internal map during transitional period.
Intended as a shared type for records and map values.
Thus no longer exposing internal map.
In preparation of introducing a shared type for record values.
blackwinter
force-pushed
the
64-introduceRecordClass
branch
from
605f116 to
c6e20f9
Compare
|
Essential functionality should be ready for review now. There are still some missing pieces (esp. unit tests and Javadoc for newly introduced APIs), but I consider them optional for the purposes of this pull request (weren't present before either). A slightly more severe issue might be that I went a little overboard with the initial assignment ;) And I did it in the wrong order, so it's not easily ripped out now. If you feel that the Also, there are probably more opportunities for consolidation/simplification that I wasn't able to investigate yet. If, however, there are no objections at all by any chance, feel free to merge right away in order to unblock further development. |
Obviates the need for `instanceof` checks and unchecked casts. A `Value` is a container/wrapper for either - an `Array` (which in turn is a wrapper for `List<Value>`), or - a `Hash` (which in turn is a wrapper for `Map<String, Value>`), or - a `String` (which is the terminal type)
- `Value.Hash` -> `hash` - `key`/`k` -> `field`/`f`
Seems to be currently unused and doesn't do what it's supposed to do. See #60 (comment).
blackwinter
force-pushed
the
64-introduceRecordClass
branch
2 times, most recently
from
4003d1b to
ba509eb
Compare
blackwinter
force-pushed
the
64-introduceRecordClass
branch
from
ba509eb to
deeff8a
Compare
|
Sorry for the noise, I'll leave it alone now! |
There was a problem hiding this comment.
Great, thanks! The Value abstraction looks good. Just irritated a bit by the naming of Array and Hash for List and Map. Also field for key, I'd consider a field a key-value pair: field name and field value. However, maybe it's good to have more specific / domain / Catmandu-related naming at this level.
|
Wow, that was fast :) Thanks!
However, maybe it's good to have more specific / domain / Catmandu-related naming at this level.
Exactly, that's what I was going for. But we can certainly discuss more.
|
Resolves #64.